pkg/payload/render_test.go (1)

67-84: Scan manifests recursively to match the “all manifests” intent.

At Line 68, os.ReadDir only checks the top level of ../../install, so nested manifest files would be skipped silently.

♻️ Proposed refactor

-	installDir := filepath.Join("../../install")
-	files, err := os.ReadDir(installDir)
+	installDir := "../../install"
+	var files []string
+	err := filepath.Walk(installDir, func(path string, info os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+		if info.IsDir() {
+			return nil
+		}
+		ext := strings.ToLower(filepath.Ext(info.Name()))
+		if ext == ".yaml" || ext == ".yml" {
+			files = append(files, path)
+		}
+		return nil
+	})
 	if err != nil {
 		t.Fatalf("failed to read directory: %v", err)
 	}
@@
-	for _, manifestFile := range files {
-		if manifestFile.IsDir() {
-			continue
-		}
-		filePath := filepath.Join(installDir, manifestFile.Name())
+	for _, filePath := range files {
 		manifests, err := manifest.ManifestsFromFiles([]string{filePath})

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/payload/render_test.go` around lines 67 - 84, The test reads installDir
using os.ReadDir which only lists the top-level entries, so nested manifests
under installDir are missed; change the directory traversal to recurse (e.g.,
use filepath.WalkDir or a recursive os.ReadDir helper) to collect all file paths
under installDir before calling manifest.ManifestsFromFiles, then filter files
as before into manifestsWithoutIncludeAnnotation using the existing prefix
constant ("include.release.openshift.io/"), ensuring you pass the full list of
discovered file paths (not just top-level entries) to
manifest.ManifestsFromFiles.

pkg/payload/render_test.go (1)

88-103: Use recursive manifest discovery to guarantee full payload coverage.

os.ReadDir(tt.dir) only checks top-level files. If manifests are later placed in nested directories, this test can miss them while still passing. Consider filepath.WalkDir with manifest-extension filtering.

♻️ Proposed refactor

+import "io/fs"
...
-			files, err := os.ReadDir(tt.dir)
+			var files []string
+			err := filepath.WalkDir(tt.dir, func(path string, d fs.DirEntry, err error) error {
+				if err != nil {
+					return err
+				}
+				if d.IsDir() {
+					return nil
+				}
+				ext := strings.ToLower(filepath.Ext(path))
+				if ext != ".yaml" && ext != ".yml" && ext != ".json" {
+					return nil
+				}
+				files = append(files, path)
+				return nil
+			})
 			if err != nil {
 				t.Fatalf("failed to read directory: %v", err)
 			}

 			if len(files) == 0 {
 				t.Fatalf("no files found in %s", tt.dir)
 			}
...
-			for _, manifestFile := range files {
-				if manifestFile.IsDir() {
-					continue
-				}
-				filePath := filepath.Join(tt.dir, manifestFile.Name())
+			for _, filePath := range files {

Also applies to: 99-103

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/payload/render_test.go` around lines 88 - 103, The test currently uses
os.ReadDir(tt.dir) which only reads top-level entries, so replace that with a
recursive walk (use filepath.WalkDir starting at tt.dir) to discover manifests
in nested directories; inside the WalkDir callback skip directories, filter
files by manifest extensions (e.g., .yaml, .yml, .json), build the same list
previously populated (manifestsWithoutIncludeAnnotation) and preserve the
existing logic that computes include annotation checks using prefix
("include.release.openshift.io/"); apply the same change for the other
occurrence noted around lines 99-103 so both checks use recursive discovery and
proper error handling in the WalkDir callback.

pkg/payload/render_test.go (1)

88-103: Use recursive traversal so this test truly covers all payload manifests.

os.ReadDir only checks one directory level, and Line 100 skips subdirectories entirely. If manifests are later placed in nested folders, they won’t be validated.

♻️ Proposed refactor

 import (
 	"bytes"
 	"fmt"
+	"io/fs"
 	"os"
 	"path/filepath"
 	"strings"
 	"testing"
@@
-			files, err := os.ReadDir(tt.dir)
-			if err != nil {
-				t.Fatalf("failed to read directory: %v", err)
-			}
-
-			if len(files) == 0 {
-				t.Fatalf("no files found in %s", tt.dir)
-			}
-
 			var manifestsWithoutIncludeAnnotation []manifest.Manifest
 			const prefix = "include.release.openshift.io/"
-			for _, manifestFile := range files {
-				if manifestFile.IsDir() {
-					continue
-				}
-				filePath := filepath.Join(tt.dir, manifestFile.Name())
-				data, err := os.ReadFile(filePath)
-				if err != nil {
-					t.Fatalf("failed to read manifest file: %v", err)
-				}
-				data, err = renderManifest(config, data)
-				if err != nil {
-					t.Fatalf("failed to render manifest: %v", err)
-				}
-				manifests, err := manifest.ParseManifests(bytes.NewReader(data))
-				if err != nil {
-					t.Fatalf("failed to load manifests: %v", err)
-				}
-
-				for _, m := range manifests {
-					m.OriginalFilename = filePath
-					var found bool
-					for k := range m.Obj.GetAnnotations() {
-						if strings.HasPrefix(k, prefix) {
-							found = true
-							break
-						}
-					}
-					if !found {
-						manifestsWithoutIncludeAnnotation = append(manifestsWithoutIncludeAnnotation, m)
-					}
-				}
-			}
+			err := filepath.WalkDir(tt.dir, func(filePath string, d fs.DirEntry, walkErr error) error {
+				if walkErr != nil {
+					return walkErr
+				}
+				if d.IsDir() {
+					return nil
+				}
+				data, err := os.ReadFile(filePath)
+				if err != nil {
+					return fmt.Errorf("read manifest file %s: %w", filePath, err)
+				}
+				data, err = renderManifest(config, data)
+				if err != nil {
+					return fmt.Errorf("render manifest %s: %w", filePath, err)
+				}
+				manifests, err := manifest.ParseManifests(bytes.NewReader(data))
+				if err != nil {
+					return fmt.Errorf("load manifests from %s: %w", filePath, err)
+				}
+				for _, m := range manifests {
+					m.OriginalFilename = filePath
+					var found bool
+					for k := range m.Obj.GetAnnotations() {
+						if strings.HasPrefix(k, prefix) {
+							found = true
+							break
+						}
+					}
+					if !found {
+						manifestsWithoutIncludeAnnotation = append(manifestsWithoutIncludeAnnotation, m)
+					}
+				}
+				return nil
+			})
+			if err != nil {
+				t.Fatalf("failed manifest scan for %s: %v", tt.dir, err)
+			}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/payload/render_test.go` around lines 88 - 103, The test currently uses
os.ReadDir on tt.dir and skips subdirectories (manifestFile.IsDir()), so nested
manifests are ignored; replace the non-recursive loop with a recursive traversal
(e.g., filepath.WalkDir) starting at tt.dir, skip directory entries inside the
WalkDirFunc, and for each file build the full path (similar to filePath :=
filepath.Join(...)) and run the same manifest parsing/annotation checks that
populate manifestsWithoutIncludeAnnotation using the same const prefix =
"include.release.openshift.io/"; ensure symlinks or non-regular files are
handled consistently with the original behavior.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes